Skip to content

text: Implement double/triple click selection #16946

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 6, 2024

Conversation

kjarosh
Copy link
Member

@kjarosh kjarosh commented Jun 30, 2024

Fixes #14996, progresses #1891.

This PR implements selecting words by double-clicking and selecting lines by triple-clicking. It also includes support for selecting words and lines while dragging.

A test is added to verify this behavior. Example use cases covered by this test:

  1. selecting characters,
  2. selecting words by double-clicking,
  3. selecting multiple words by double-clicking and dragging,
  4. selecting lines by triple-clicking,
  5. selecting multiple lines by triple-clicking and dragging,
  6. double-clicking at word boundaries,
  7. double-clicking between spaces,
  8. changing the underlying text while clicking and dragging,
  9. trying to select while the text field is not selectable.

@kjarosh kjarosh force-pushed the edittext-select-words branch 2 times, most recently from 7be1773 to 1a8d716 Compare June 30, 2024 23:20
@kjarosh kjarosh added text Issues relating to text rendering/input waiting-on-review Waiting on review from a Ruffle team member labels Jun 30, 2024
@Dinnerbone
Copy link
Contributor

All that and tests?! You spoil us!

@torokati44
Copy link
Member

I want quadruple click selection!! 😜

@kjarosh
Copy link
Member Author

kjarosh commented Jun 30, 2024

Unfortunately the test textbox_click did not take into account that FP supports double/triple click and always expects to see a caret instead of selecting words/lines. That needs to be fixed

/// Maximum duration between two clicks for them to be considered as a multi-click.
///
/// Derived experimentally to be 250 ms.
const MULTI_CLICK_MAX_DURATION: TimeDelta = TimeDelta::milliseconds(250);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That value is pretty low, usually 500 ms is used. I'm wondering whether in the future we should add an option to increase that as an accessibility setting.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to be 500ms for me in FP on Windows

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this affected by the setting mentioned on https://en.m.wikipedia.org/wiki/Double-click#Speed_and_timing ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good shout - yes it is.

Copy link
Contributor

@Dinnerbone Dinnerbone Jul 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I suspect this is actually not handled by edittext, but a general player event "double click" - which edittext listens to. Triple click is probably custom though.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's likely.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That seems to be platform-dependent then, because on GNOME it's constant 250 ms irrespective of double click delay setting.

Unfortunately it seems that winit does not support double clicking though. Also, I wouldn't be able to test the behavior of triple clicking as I cannot change the delay 😐

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I changed the delay to 500ms and added a TODO. I will address supporting double clicks at OS level in another PR. It should be easy to implement on the web, but on desktop I'm afraid we will need to add a setting until it's implemented in winit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, I implemented a rough framework for double clicks, so that the click index may be supplied in player events, and when it's not, it's calculated.

Currently it's not supplied and TODOs are added both in desktop and web (tried js_event.detail but it doesn't work OOTB).

@kjarosh kjarosh force-pushed the edittext-select-words branch 7 times, most recently from 2c0eb7e to d82e9b1 Compare July 5, 2024 12:59
Copy link
Contributor

@Dinnerbone Dinnerbone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

kjarosh added 7 commits July 6, 2024 18:20
This patch implements selecting words by double-clicking and
selecting lines by triple-clicking.
It also includes support for selecting words and lines while dragging.
This test verifies the behavior of selecting text using mouse.
It takes into account normal, word, and line selection.
Example use cases covered by this test:
1. selecting characters,
2. selecting words by double-clicking,
3. selecting multiple words by double-clicking and dragging,
4. selecting lines by triple-clicking,
5. selecting multiple lines by triple-clicking and dragging,
6. double-clicking at word boundaries,
7. double-clicking between spaces,
8. changing the underlying text while clicking and dragging,
9. trying to select while the text field is not selectable.
@Dinnerbone Dinnerbone force-pushed the edittext-select-words branch from d82e9b1 to b86d437 Compare July 6, 2024 16:20
@Dinnerbone Dinnerbone enabled auto-merge (rebase) July 6, 2024 16:20
@Dinnerbone Dinnerbone merged commit 9d7629e into ruffle-rs:master Jul 6, 2024
17 checks passed
@kjarosh kjarosh deleted the edittext-select-words branch July 6, 2024 17:04
@kjarosh kjarosh removed the waiting-on-review Waiting on review from a Ruffle team member label Jul 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
newsworthy text Issues relating to text rendering/input
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

TextFields mouse control for text select
4 participants